tests/unittest/_torch/attention/sparse/test_minimax_m3_msa_backend.py (2)

47-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Missing positive-case coverage for is_available.

Only the negative cases (None, object()) are tested. Consider adding an assertion that MsaSparseGqaFmha.is_available(<actual MSA attention instance>) returns True, to confirm the filter actually admits the intended layer type, not just rejects others.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unittest/_torch/attention/sparse/test_minimax_m3_msa_backend.py` around
lines 47 - 51, Add positive coverage to
test_msa_fmha_is_available_filters_on_owning_attention by constructing the
actual MSA attention instance used by this backend and asserting
MsaSparseGqaFmha.is_available(...) returns True, while preserving the existing
None and non-MSA rejection assertions.

Source: Path instructions

---

135-140: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Test doesn't verify the claimed rejection behavior.

The test name and comment claim "a mismatch must fail loudly," but the assertions only confirm params.topk preserves the passed-through value — no failure is actually exercised or expected here. As written, this test would pass even if the backend never gates bad topk values. Consider either renaming the test to reflect what it actually checks (config lowering is pass-through), or adding an assertion at the point where gating is supposed to occur (e.g. instantiating/using the MSA backend with topk=8 and expecting a ValueError/RuntimeError).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unittest/_torch/attention/sparse/test_minimax_m3_msa_backend.py` around
lines 135 - 140, Update test_msa_params_reject_bad_topk_at_construction to
verify the claimed rejection at the MSA backend construction or usage point,
asserting the expected ValueError or RuntimeError for sparse_topk_blocks=8;
alternatively, rename the test and comments to accurately describe that
MiniMaxM3SparseAttentionConfig.to_sparse_params preserves the value without
testing backend validation.

Source: Path instructions

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

tensorrt_llm/_torch/attention_backend/sparse/minimax_m3/triton_metadata.py (1)

419-651: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Dead static_buffers branches after the early return.

Since Lines 422-432 return via _build_runtime_metadata_fresh(...) whenever static_buffers is None, every subsequent if static_buffers is not None: guard (Lines 442, 504, 526, 557, 577, 631) is always true and each corresponding else branch (Lines 515-516, 538-540, 562-565, 606-610, 641-642) is unreachable. This dead code makes an already dense function harder to follow. Drop the redundant guards and their else branches so the remaining body reads as the static-buffers-only path it now is.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tensorrt_llm/_torch/attention_backend/sparse/minimax_m3/triton_metadata.py`
around lines 419 - 651, Remove the unreachable non-static branches after the
early return in the metadata-building flow. In the function containing
_build_runtime_metadata_fresh, simplify each subsequent static_buffers guard to
its static-buffer path, removing the corresponding else branches and redundant
checks while preserving the existing allocation, copying, and metadata
construction behavior.

tensorrt_llm/llmapi/llm_args.py (1)

752-758: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add concrete method annotations.

The new validator and metadata-lowering method are unannotated, including the untyped **kwargs boundary. Add explicit parameter and return types without introducing Any.

As per coding guidelines, “Annotate every function” and “avoid Any.”

Also applies to: 782-803

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tensorrt_llm/llmapi/llm_args.py` around lines 752 - 758, Add explicit type
annotations to _validate_msa_block_size and the metadata-lowering method in the
same class, including a concrete type for the **kwargs boundary and each return
type. Use existing project-specific types or precise mappings rather than Any,
while preserving the validator’s current behavior.

Source: Coding guidelines

tensorrt_llm/_torch/attention_backend/sparse/minimax_m3/msa_backend.py (1)

55-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Narrow the blind except Exception: pass.

Catching bare Exception and swallowing it silently masks unexpected failures in get_buffers(0) and violates the repo's exception-handling guideline. Catch the specific error you expect (e.g. AttributeError/RuntimeError) so unrelated failures still surface.

As per coding guidelines: "Avoid broad exception handling; catch specific exceptions instead of using bare except:" and "Catch the narrowest possible exceptions".

♻️ Proposed narrowing

     if kv_cache_manager is not None:
         try:
             return kv_cache_manager.get_buffers(0).device
-        except Exception:
+        except (AttributeError, RuntimeError):
             pass
     return torch.device(f"cuda:{torch.cuda.current_device()}")

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tensorrt_llm/_torch/attention_backend/sparse/minimax_m3/msa_backend.py`
around lines 55 - 60, In the device-selection logic around
kv_cache_manager.get_buffers(0), replace the broad silent Exception handler with
only the specific expected exception types, such as AttributeError or
RuntimeError, that indicate the buffer device cannot be retrieved. Preserve the
fallback to torch.device using the current CUDA device, while allowing unrelated
failures to propagate.

Sources: Coding guidelines, Linters/SAST tools

tensorrt_llm/_torch/pyexecutor/config_utils.py (1)

99-101: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add type annotations.

Annotate the parameter and -> bool return; the neighboring helpers in this module use forward-ref typed params.

As per coding guidelines: "Annotate every function, use None for non-returning functions".

♻️ Proposed annotation

-def is_minimax_m3(sparse_attention_config):
+def is_minimax_m3(sparse_attention_config: Optional["SparseAttentionConfig"]) -> bool:
     """True when the sparse attention config selects the MiniMax-M3 algorithm."""
     return sparse_attention_config is not None and sparse_attention_config.algorithm == "minimax_m3"

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tensorrt_llm/_torch/pyexecutor/config_utils.py` around lines 99 - 101, Update
the is_minimax_m3 function signature to add a forward-reference type annotation
for sparse_attention_config and an explicit bool return annotation, matching the
neighboring helper functions’ typing style while preserving its existing
behavior.

Source: Coding guidelines

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

tensorrt_llm/_torch/attention_backend/sparse/minimax_m3/triton_metadata.py (1)

419-432: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Early return leaves the inline fresh-allocation branches dead.

Since this new guard returns via _build_runtime_metadata_fresh whenever static_buffers is None, every if static_buffers is not None: check further down (L442, L504, L526, L557, L577, L631) is now always-true, and each else fresh-allocation branch (e.g. L515-516, L538-540, L562-565, L606-610, L641-642) is unreachable. That dead path duplicates _build_runtime_metadata_fresh and will silently diverge from it over time. Consider dropping the redundant guards/else branches so the static-buffer path reads unconditionally.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tensorrt_llm/_torch/attention_backend/sparse/minimax_m3/triton_metadata.py`
around lines 419 - 432, Remove the redundant static_buffers conditionals and
unreachable fresh-allocation else branches throughout the metadata-building flow
after the early return in the surrounding function. Make the remaining logic
unconditionally use static buffers, while retaining
_build_runtime_metadata_fresh as the sole path for static_buffers=None and
preserving the existing static-buffer behavior.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes